Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new type support in query_as #2369

Merged
merged 3 commits into from
Mar 9, 2023
Merged

new type support in query_as #2369

merged 3 commits into from
Mar 9, 2023

Conversation

0xdeafbeef
Copy link
Contributor

fixes #2173

use sqlx::PgPool;


struct Wrap(bigdecimal::BigDecimal);

impl From<bigdecimal::BigDecimal> for Wrap {
    fn from(x: bigdecimal::BigDecimal) -> Self {
        Wrap(x)
    }
}

impl From<Wrap> for bigdecimal::BigDecimal {
    fn from(x: Wrap) -> Self {
        x.0
    }
}

struct Data {
    pub amount: Wrap,
}


#[tokio::main]
async fn main() {
    let db = PgPool::connect("postgres://postgres:mysecretpassword@localhost:5432/teststore").await.unwrap();

    sqlx::query_as!(Data, "select amount from test").fetch_one(&db).await.unwrap();
    // previously it will expand like this:
     {
         {
             #[allow(clippy::all)] {
                 use ::sqlx::Arguments   as _;
                 let query_args = <sqlx::postgres::Postgres as ::sqlx::database::HasArguments>::Arguments::default();
                 ::sqlx::query_with::<sqlx::postgres::Postgres, _>("select amount from test", query_args).try_map(|row: sqlx::postgres::PgRow| {
                     use ::sqlx::Row   as _;
                     let sqlx_query_as_amount = row.try_get_unchecked::<sqlx::types::BigDecimal, _>(0usize)?;
                     Ok(Data { r#amount: sqlx_query_as_amount })
                 })
             }
         }
     }
 now it will expand like this:
let sqlx_query_as_amount = row.try_get_unchecked::<sqlx::types::BigDecimal, _>(0usize)?.into(); <-----  

This change shouldn't break anything, because From<T> is implemented for T.

Maybe we can also use From for binded arguments in sqlx::query

example:

let t = "string";
sqlx::query!("select $1",  t)

Here we can use t.into() internally, but it can break inference

@0xdeafbeef 0xdeafbeef changed the title fix: sqlx-postgres bigdecimal feature new type support in query_as Feb 23, 2023
@0xdeafbeef 0xdeafbeef force-pushed the main branch 2 times, most recently from 4166e9f to f99264f Compare February 23, 2023 11:40
@abonander
Copy link
Collaborator

There was a reason I didn't do this and I believe it had to do with compile errors. Can you test what happens when you try to use an incompatible type?

@0xdeafbeef
Copy link
Contributor Author

There was a reason I didn't do this and I believe it had to do with compile errors. Can you test what happens when you try to use an incompatible type?

   |
27 |     sqlx::query_as!(Record2,r#"select 1 as "amount!""#).fetch_one(&db).await.unwrap();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<i32>` is not implemented for `std::string::String`
   |
   = help: the following other types implement trait `From<T>`:
             <std::string::String as From<&mut str>>
             <std::string::String as From<&std::string::String>>
             <std::string::String as From<&str>>
             <std::string::String as From<Box<str>>>
             <std::string::String as From<Cow<'a, str>>>
             <std::string::String as From<char>>
             <std::string::String as From<url::Url>>
   = note: required for `i32` to implement `Into<std::string::String>`
   = note: this error originates in the macro `$crate::sqlx_macros::expand_query` which comes from the expansion of the macro `sqlx::query_as` (in Nightly builds, run with -Z macro-backtrace for more info)

I'll try to add some type checking to make errors more obvious.

The current error looks like this:

  --> src/main.rs:27:5
   |
27 |     sqlx::query_as!(Record2, r#"select 1 as "amount!""#).fetch_one(&db).await.unwrap();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |     |
   |     expected struct `std::string::String`, found `i32`
   |     help: try using a conversion method: `.to_string()`
   |
   = note: this error originates in the macro `$crate::sqlx_macros::expand_query` which comes from the expansion of the macro `sqlx::query_as` (in Nightly builds, run with -Z macro-backtrace for more info)

@abonander
Copy link
Collaborator

Yeah, neither error is great. Something I want to do shortly but won't make the 0.7.0 release is rewrite the typechecking code entirely so that it also works with FromRow. I figure I can use panic!() messages in const to give much more useful error messages.

@CosmicHorrorDev
Copy link
Contributor

rewrite the typechecking code entirely so that it also works with FromRow. I figure I can use panic!() messages in const to give much more useful error messages.

Yes please! Both using FromRow and giving better error messages would be amazing!

@abonander
Copy link
Collaborator

The annoying part is that panic!() in const doesn't support formatting AFAIK, so I'd have to use a dirty hack like the following: https://github.com/Vurich/const-concat

As-implemented, I think that macro would trigger #[deny(unsafe_code)] if the user has it set. I think it can be cleaned up with const-generics though.

@abonander
Copy link
Collaborator

abonander commented Mar 4, 2023

I suppose concatenation isn't really necessary as I could just emit two different consts per check, e.g.:

const _: () = panic!("field `foo` of struct `Bar` expected type `i32`");
const _: () = panic!("column `foo` of query has SQL type `TEXT`");

gives something like:

   Compiling playground v0.0.1 (/playground)
error[[E0080]](https://doc.rust-lang.org/stable/error_codes/E0080.html): evaluation of constant value failed
 --> src/lib.rs:1:15
  |
1 | const _: () = panic!("field `foo` of struct `Bar` expected type `i32`");
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'field `foo` of struct `Bar` expected type `i32`', src/lib.rs:1:15
  |
  = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[[E0080]](https://doc.rust-lang.org/stable/error_codes/E0080.html): evaluation of constant value failed
 --> src/lib.rs:2:15
  |
2 | const _: () = panic!("column `foo` of query has SQL type `TEXT`");
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'column `foo` of query has SQL type `TEXT`', src/lib.rs:2:15
  |
  = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0080`.
error: could not compile `playground` due to 2 previous errors

The formatting isn't ideal but at least all the relevant information is there.

@abonander abonander merged commit 5a8cd95 into launchbadge:main Mar 9, 2023
@heksesang
Copy link

Is this feature supposed to work for Options?

@beyarkay
Copy link

For future travellers, these sqlx docs solved my problem. Basically:

#[derive(sqlx::Type)]
#[sqlx(transparent)]
struct UserId(i64);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use derived transparent Type as parameter
5 participants